Skip to content

Remove INAV from Position Control #30037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

Remove INAV from Position Control #30037

wants to merge 33 commits into from
+319 −255

Conversation

lthall
Copy link
Contributor

@lthall lthall commented May 11, 2025 •
edited
Loading

This PR removes INAV from the position controller and replaces many functions in the main code to directly reference the position controller state where approriate.

Board,blimp,bootloader,copter,heli,plane,rover,sub
CubeOrange,608,*,1008,1024,688,760,648

@lthall lthall requested a review from rmackay9 May 11, 2025 14:08
@IamPete1
Copy link
Member

On plane this make me very nervous that we will forget to call update_estimates, I think the current plane changes don't get them all, and going forward it will be hard to not introduce bugs.

What is the goal or removing inav? It is still used for loiter and circle so this change makes it possible this users are have a different value the pos controller.

Why not add the call pos_control.update_estimates where inertial_nav.update is called now? It would be less code, and remove the possibility of forgetting to call it, the disadvantage would be a little more computation as it would be called when its not used.

@lthall lthall force-pushed the 20250506_postype-through-ahrs branch from 6b01897 to c0023b8 Compare May 11, 2025 23:15
@lthall
Copy link
Contributor Author

lthall commented May 11, 2025

On plane this make me very nervous that we will forget to call update_estimates, I think the current plane changes don't get them all, and going forward it will be hard to not introduce bugs.

What is the goal or removing inav? It is still used for loiter and circle so this change makes it possible this users are have a different value the pos controller.

Why not add the call pos_control.update_estimates where inertial_nav.update is called now? It would be less code, and remove the possibility of forgetting to call it, the disadvantage would be a little more computation as it would be called when its not used.

You are completly correct. Doing it now.

@lthall lthall force-pushed the 20250506_postype-through-ahrs branch 4 times, most recently from 2f3146b to 2c4f73f Compare May 12, 2025 03:56
rmackay9
rmackay9 reviewed May 12, 2025
View reviewed changes
@lthall lthall force-pushed the 20250506_postype-through-ahrs branch 2 times, most recently from 202447f to e2164d4 Compare May 12, 2025 08:15
@lthall
Copy link
Contributor Author

lthall commented May 12, 2025 •
edited
Loading

On plane this make me very nervous that we will forget to call update_estimates, I think the current plane changes don't get them all, and going forward it will be hard to not introduce bugs.
What is the goal or removing inav? It is still used for loiter and circle so this change makes it possible this users are have a different value the pos controller.
Why not add the call pos_control.update_estimates where inertial_nav.update is called now? It would be less code, and remove the possibility of forgetting to call it, the disadvantage would be a little more computation as it would be called when its not used.

You are completly correct. Doing it now.

This has resulted in a crash dump but I don't understand why.

Fixed

IamPete1
IamPete1 reviewed May 12, 2025
View reviewed changes
@lthall lthall force-pushed the 20250506_postype-through-ahrs branch from e2164d4 to 7d03568 Compare May 12, 2025 22:53
@peterbarker @lthall
AC_Avoidance: rename get_relative_position_NED_origin methods to incl…
b6c2184 
…ude 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AC_PrecLand: rename get_relative_position_NED_origin methods to inclu…
0108cd3 
…de 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AP_AHRS: rename get_relative_position_NED_origin methods to include '…
7253faf 
…float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AP_Follow: rename get_relative_position_NED_origin methods to include…
84bc6be 
… 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AP_ICEngine: rename get_relative_position_NED_origin methods to inclu…
a0173da 
…de 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AP_InertialNav: rename get_relative_position_NED_origin methods to in…
1a151e1 
…clude 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
APM_Control: rename get_relative_position_NED_origin methods to inclu…
2975dc8 
…de 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AP_Module: rename get_relative_position_NED_origin methods to include…
a855490 
… 'float'

can't have both postype and these methods, something has to shift names
peterbarker and others added 12 commits May 13, 2025 08:30
@peterbarker @lthall
AP_VisualOdom: rename get_relative_position_NED_origin methods to inc…
c0dd0d9 
…lude 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AR_WPNav: rename get_relative_position_NED_origin methods to include …
77f185e 
…'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
GCS_MAVLink: rename get_relative_position_NED_origin methods to inclu…
e6fc218 
…de 'float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
ArduSub: rename get_relative_position_NED_origin methods to include '…
733c61c 
…float'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
Blimp: rename get_relative_position_NED_origin methods to include 'fl…
892a16c 
…oat'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
Rover: rename get_relative_position_NED_origin methods to include 'fl…
a3976e7 
…oat'

can't have both postype and these methods, something has to shift names
@peterbarker @lthall
AP_AHRS: allow AHRS to return postype for position-relative-to-origin
46f3d8c
@peterbarker @lthall
AP_Common: allow AHRS to return postype for position-relative-to-origin
b9eb6a7
@peterbarker @lthall
AP_NavEKF2: allow AHRS to return postype for position-relative-to-origin
d72bab6
@peterbarker @lthall
AP_NavEKF3: allow AHRS to return postype for position-relative-to-origin
626f1c5
@lthall
AC_PID: AC_P_+2D: use Vector2p input
830ba07 
squash PID
@lthall
APM_Control: Use NED Vector2p input to AC_P_2D
2a98b22
@lthall lthall force-pushed the 20250506_postype-through-ahrs branch 2 times, most recently from 4f8848a to 85c80c1 Compare May 13, 2025 11:05
@lthall lthall force-pushed the 20250506_postype-through-ahrs branch from 85c80c1 to 0c360f9 Compare May 13, 2025 12:04
@lthall
Copy link
Contributor Author

lthall commented May 13, 2025

Is this the right name?

get_pos_NEU_cm
get_pos_current_NEU_cm
get_pos_estimate_NEU_cm

I think I like get_pos_estimate_NEU_cm

@lthall lthall force-pushed the 20250506_postype-through-ahrs branch from 0c360f9 to c1efd81 Compare May 13, 2025 14:03
@lthall lthall requested review from rmackay9 and IamPete1 May 15, 2025 10:48
@lthall lthall force-pushed the 20250506_postype-through-ahrs branch from c1efd81 to 7b69232 Compare May 15, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmackay9 rmackay9 Awaiting requested review from rmackay9

@IamPete1 IamPete1 Awaiting requested review from IamPete1

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Copter DevCallTopic Plane Rover Sub
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lthall @IamPete1 @rmackay9 @peterbarker